Skip to content

Adapted SVD API to match numpy API #2089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
brownbaerchen merged 5 commits into from
Jan 14, 2026
Merged

Adapted SVD API to match numpy API #2089

brownbaerchen merged 5 commits into from
Jan 14, 2026

Conversation

@brownbaerchen
Copy link
Collaborator

@brownbaerchen brownbaerchen commented Jan 7, 2026 •
edited
Loading

Due Diligence

  • General:
  • Implementation:
    • unit tests: all split configurations tested
    • unit tests: multiple dtypes tested
    • NEW unit tests: MPS tested (1 MPI process, 1 GPU)
    • benchmarks: created for new functionality
    • benchmarks: performance improved or maintained
    • documentation updated where needed

Description

As detailed in #2054, the APIs of heat and numpy differ for svd in that heat returns $V$ and numpy returns $V^T$ in the decomposition $A = U S V^T$. This PR changes the heat API for svd and related functions to match the numpy API.

Issue/s resolved: #2054

Changes proposed:

  • Breaking API change: svd and related functions return $V^T$ instead of $V$ as before
  • Small refactor of heat.core.linalg.svd
  • Add test that the heat and numpy svd APIs match
  • Fix a few small typos in documentation

Type of change

  • Bug fix
  • Breaking change

@github-project-automation github-project-automation bot moved this to Todo in Roadmap Jan 7, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

Thank you for the PR!

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

Thank you for the PR!

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

Thank you for the PR!

@JuanPedroGHM
Copy link
Member

JuanPedroGHM commented Jan 7, 2026 •
edited
Loading

Benchmarks results - Sponsored by perun

FTPx86-AMD-Milan-Mi250 - CPU - N4 - RUNTIME

Function Value [s] Change % 30 day avg. [s] Last entrie [s]
randomized_eigh_split0 0.0168619 35.2121 0.0124707 0.0124149
randomized_eigh_split1 0.0170561 -10.036 0.0189588 0.0224528
apply_inplace_min_max_scaler_and_inverse 0.00502672 12.2196 0.00447936 0.00508194
apply_inplace_max_abs_scaler_and_inverse 0.0038168 129.064 0.00166626 0.00193071
apply_inplace_normalizer 0.00843201 22.4499 0.00688609 0.00569382

FTPx86-AMD-Milan-Mi250 - GPU - N4 - RUNTIME

Function Value [s] Change % 30 day avg. [s] Last entrie [s]
randomized_eigh_split0 0.060691 -12.9349 0.0697075 0.0652272
apply_inplace_min_max_scaler_and_inverse 0.00230427 -52.3606 0.0048369 0.00637541
apply_inplace_max_abs_scaler_and_inverse 0.00108938 -49.0937 0.00213998 0.00125761
apply_inplace_normalizer 0.00925636 -14.1995 0.0107882 0.0116171
convolution_array_distributed 0.0147165 -14.575 0.0172274 0.0174377
convolution_array_distributed_stride 0.0254059 -21.5148 0.0323703 0.0386677
convolution_batch_processing 0.0028965 -43.3337 0.0051115 0.00678673

Grafana Dashboard
Last updated: 2026-01-14T13:38:10Z

@codecov
Copy link

codecov bot commented Jan 7, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 98.50746% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.68%. Comparing base (7e2a0ce) to head (6795d5b).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
heat/core/linalg/svdtools.py 94.11% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2089      +/-   ##
==========================================
+ Coverage   88.73%   91.68%   +2.94%     
==========================================
  Files          89       89              
  Lines       13942    13945       +3     
==========================================
+ Hits        12372    12786     +414     
+ Misses       1570     1159     -411
Flag Coverage Δ
unit 91.68% <98.50%> (+2.94%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@brownbaerchen brownbaerchen requested a review from mrfh92 January 12, 2026 09:16
@ClaudiaComito ClaudiaComito added this to the 1.7.1 milestone Jan 12, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 14, 2026

Thank you for the PR!

mrfh92
mrfh92 approved these changes Jan 14, 2026
View reviewed changes
Copy link
Collaborator

@mrfh92 mrfh92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have already discussed this issue in some PR talk last year. Although I do not understand why one should return Vt instead of V, I see that we should aim at consistency with numpy etc. (Also LAPACK seems to use Vt, so there might indeed be some good reason I dont know...).

@brownbaerchen thanks for taking this over! 👍

@github-project-automation github-project-automation bot moved this from Todo to Merge queue in Roadmap Jan 14, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 14, 2026

Thank you for the PR!

@brownbaerchen brownbaerchen merged commit 4dddb5b into main Jan 14, 2026
11 checks passed
@github-project-automation github-project-automation bot moved this from Merge queue to Done in Roadmap Jan 14, 2026
@brownbaerchen brownbaerchen deleted the bugs/2054-_Bug_SVD_API_inconsistencies branch January 14, 2026 14:41
@github-actions
Copy link
Contributor

github-actions bot commented Jan 14, 2026

Successfully created backport PR for stable:

@github-actions github-actions bot mentioned this pull request Jan 14, 2026
Open
@brownbaerchen brownbaerchen mentioned this pull request Jan 16, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrfh92 mrfh92 mrfh92 approved these changes

Assignees

No one assigned

Labels

backport stable benchmark PR bug Something isn't working core decomposition linalg

Projects

Roadmap
Status: Done

Milestone

1.7.1

Development

Successfully merging this pull request may close these issues.

[Bug]: SVD API inconsistencies

5 participants

@brownbaerchen @JuanPedroGHM @mrfh92 @ClaudiaComito